Various optimizations #2
Merged
+662
−459
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
No-op metrics implementation for benchmarking etc. Signed-off-by: Jussi Maki <[email protected]>
- Do not use require.Equal etc. in tight loop as it's slow - Use a constant data set size to avoid using way way too much memory - Use nop metrics - Add RandomReplace benchmark and make RandomInsert always insert new objects. Signed-off-by: Jussi Maki <[email protected]>
WriteTxn() is fairly slow and partially the reason is the heaviness of using hash maps to look up the tables that are being modified. Since the set of tables is static at runtime we can just use integer index to look up. This can later be extended to the indexTxn as well. Before: goos: linux goarch: amd64 pkg: github.com/cilium/statedb cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz BenchmarkDB_WriteTxn_1-8 132523 9268 ns/op 107899 objects/sec BenchmarkDB_WriteTxn_10-8 367114 3044 ns/op 328531 objects/sec BenchmarkDB_WriteTxn_100-8 430207 2459 ns/op 406751 objects/sec BenchmarkDB_WriteTxn_100_SecondaryIndex-8 503071 2358 ns/op 424111 objects/sec BenchmarkDB_RandomInsert-8 945 1288636 ns/op 776014 objects/sec BenchmarkDB_RandomReplace-8 226 5256999 ns/op 190223 objects/sec BenchmarkDB_SequentialInsert-8 370 3580799 ns/op 279267 objects/sec ... BenchmarkDB_DeleteTracker_Baseline-8 356 3314962 ns/op 301663 objects/sec BenchmarkDB_DeleteTracker-8 181 6609844 ns/op 151290 objects/sec BenchmarkDB_RandomLookup-8 3289 354713 ns/op 2819181 objects/sec BenchmarkDB_SequentialLookup-8 3519 334955 ns/op 2985479 objects/sec BenchmarkDB_FullIteration_All-8 88718 12817 ns/op 78022832 objects/sec BenchmarkDB_FullIteration_Get-8 71965 15974 ns/op 62599999 objects/sec BenchmarkDB_PropagationDelay-8 188659 6398 ns/op 55.00 50th_µs 74.00 90th_µs 277.0 99th_µs PASS ok github.com/cilium/statedb 33.132s After: goos: linux goarch: amd64 pkg: github.com/cilium/statedb cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz BenchmarkDB_WriteTxn_1-8 160879 7232 ns/op 138271 objects/sec BenchmarkDB_WriteTxn_10-8 442489 2597 ns/op 385104 objects/sec BenchmarkDB_WriteTxn_100-8 511983 2201 ns/op 454271 objects/sec BenchmarkDB_WriteTxn_100_SecondaryIndex-8 511678 2205 ns/op 453489 objects/sec BenchmarkDB_RandomInsert-8 1045 1134981 ns/op 881072 objects/sec BenchmarkDB_RandomReplace-8 246 4879081 ns/op 204957 objects/sec BenchmarkDB_SequentialInsert-8 387 3108060 ns/op 321744 objects/sec ... BenchmarkDB_DeleteTracker_Baseline-8 374 3167598 ns/op 315697 objects/sec BenchmarkDB_DeleteTracker-8 182 6409534 ns/op 156018 objects/sec BenchmarkDB_RandomLookup-8 3505 317084 ns/op 3153747 objects/sec BenchmarkDB_SequentialLookup-8 3951 293740 ns/op 3404371 objects/sec BenchmarkDB_FullIteration_All-8 98962 12085 ns/op 82749863 objects/sec BenchmarkDB_FullIteration_Get-8 81453 14711 ns/op 67978410 objects/sec BenchmarkDB_PropagationDelay-8 206851 5742 ns/op 50.00 50th_µs 64.00 90th_µs 261.0 99th_µs PASS ok github.com/cilium/statedb 31.966s Signed-off-by: Jussi Maki <[email protected]>
dylandreimerink
approved these changes
Mar 25, 2024
bimmlerd
approved these changes
Mar 25, 2024
| @@ -460,17 +434,18 @@ func (txn *txn) Commit() { | |||
| // Since the root may have changed since the pointer was last read in WriteTxn(), | |||
| // load it again and modify the latest version that we now have immobilised by | |||
| // the root lock. | |||
| rootTxn := db.root.Load().Txn() | |||
| root := *db.root.Load() | |||
There was a problem hiding this comment.
Why is it okay to modify this root?
EDIT: aha. was reviewing commit by commit - I guess the next line should be in an earlier commit
There was a problem hiding this comment.
I'm not following. How would you change this?
There was a problem hiding this comment.
in 308c157 we do
root := *db.root.Load()
for pos, table := range txn.modifiedTables {
if table != nil {
root[pos] = *table
}
}
which seems to modify the root in place as part of Commit, which I was confused as to why it's safe (can't other read transactions have the same root slice pointer?).
But then I saw that the next commit adds the Clone: 1858d6b#diff-0e4a5bf98fe17be3b8b206da2133e0a481d6c82fb40e8c0155f40479e26ace07R446 so I think we're good, but it seems like this line should be part of the earlier commit.
Similarly to tables we have few enough indexes per table to not require anything more complex than an array for storing them. Before: goos: linux goarch: amd64 pkg: github.com/cilium/statedb cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz BenchmarkDB_WriteTxn_1-8 160879 7232 ns/op 138271 objects/sec BenchmarkDB_WriteTxn_10-8 442489 2597 ns/op 385104 objects/sec BenchmarkDB_WriteTxn_100-8 511983 2201 ns/op 454271 objects/sec BenchmarkDB_WriteTxn_100_SecondaryIndex-8 511678 2205 ns/op 453489 objects/sec BenchmarkDB_RandomInsert-8 1045 1134981 ns/op 881072 objects/sec BenchmarkDB_RandomReplace-8 246 4879081 ns/op 204957 objects/sec BenchmarkDB_SequentialInsert-8 387 3108060 ns/op 321744 objects/sec ... BenchmarkDB_DeleteTracker_Baseline-8 374 3167598 ns/op 315697 objects/sec BenchmarkDB_DeleteTracker-8 182 6409534 ns/op 156018 objects/sec BenchmarkDB_RandomLookup-8 3505 317084 ns/op 3153747 objects/sec BenchmarkDB_SequentialLookup-8 3951 293740 ns/op 3404371 objects/sec BenchmarkDB_FullIteration_All-8 98962 12085 ns/op 82749863 objects/sec BenchmarkDB_FullIteration_Get-8 81453 14711 ns/op 67978410 objects/sec BenchmarkDB_PropagationDelay-8 206851 5742 ns/op 50.00 50th_µs 64.00 90th_µs 261.0 99th_µs PASS ok github.com/cilium/statedb 31.966s After: goos: linux goarch: amd64 pkg: github.com/cilium/statedb cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz BenchmarkDB_WriteTxn_1-8 240938 4752 ns/op 210459 objects/sec BenchmarkDB_WriteTxn_10-8 502363 2551 ns/op 392063 objects/sec BenchmarkDB_WriteTxn_100-8 457850 2279 ns/op 438872 objects/sec BenchmarkDB_WriteTxn_100_SecondaryIndex-8 526416 2222 ns/op 450100 objects/sec BenchmarkDB_RandomInsert-8 1012 1181665 ns/op 846264 objects/sec BenchmarkDB_RandomReplace-8 216 5048896 ns/op 198063 objects/sec BenchmarkDB_SequentialInsert-8 398 2996997 ns/op 333667 objects/sec ... BenchmarkDB_DeleteTracker_Baseline-8 390 3036951 ns/op 329278 objects/sec BenchmarkDB_DeleteTracker-8 141 8194663 ns/op 122031 objects/sec BenchmarkDB_RandomLookup-8 8846 134745 ns/op 7421428 objects/sec BenchmarkDB_SequentialLookup-8 8425 123284 ns/op 8111372 objects/sec BenchmarkDB_FullIteration_All-8 103279 10996 ns/op 90941891 objects/sec BenchmarkDB_FullIteration_Get-8 84451 13637 ns/op 73328686 objects/sec BenchmarkDB_PropagationDelay-8 235146 5342 ns/op 48.00 50th_µs 57.00 90th_µs 215.0 99th_µs PASS ok github.com/cilium/statedb 31.480s Signed-off-by: Jussi Maki <[email protected]>
Drop callerPackage as it was too costly and replace it with a named handle that we can use in metrics etc. The named handle can then be provided automatically with hive's module provider for per-module transaction cost accounting. Before: goos: linux goarch: amd64 pkg: github.com/cilium/statedb cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz BenchmarkDB_WriteTxn_1-8 240938 4752 ns/op 210459 objects/sec BenchmarkDB_WriteTxn_10-8 502363 2551 ns/op 392063 objects/sec BenchmarkDB_WriteTxn_100-8 457850 2279 ns/op 438872 objects/sec BenchmarkDB_WriteTxn_100_SecondaryIndex-8 526416 2222 ns/op 450100 objects/sec BenchmarkDB_RandomInsert-8 1012 1181665 ns/op 846264 objects/sec BenchmarkDB_RandomReplace-8 216 5048896 ns/op 198063 objects/sec BenchmarkDB_SequentialInsert-8 398 2996997 ns/op 333667 objects/sec ... BenchmarkDB_DeleteTracker_Baseline-8 390 3036951 ns/op 329278 objects/sec BenchmarkDB_DeleteTracker-8 141 8194663 ns/op 122031 objects/sec BenchmarkDB_RandomLookup-8 8846 134745 ns/op 7421428 objects/sec BenchmarkDB_SequentialLookup-8 8425 123284 ns/op 8111372 objects/sec BenchmarkDB_FullIteration_All-8 103279 10996 ns/op 90941891 objects/sec BenchmarkDB_FullIteration_Get-8 84451 13637 ns/op 73328686 objects/sec BenchmarkDB_PropagationDelay-8 235146 5342 ns/op 48.00 50th_µs 57.00 90th_µs 215.0 99th_µs PASS ok github.com/cilium/statedb 31.480s After: goos: linux goarch: amd64 pkg: github.com/cilium/statedb cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz BenchmarkDB_WriteTxn_1-8 310290 3885 ns/op 257388 objects/sec BenchmarkDB_WriteTxn_10-8 523450 2441 ns/op 409679 objects/sec BenchmarkDB_WriteTxn_100-8 538578 2219 ns/op 450628 objects/sec BenchmarkDB_WriteTxn_100_SecondaryIndex-8 515170 2156 ns/op 463816 objects/sec BenchmarkDB_RandomInsert-8 1110 1081693 ns/op 924477 objects/sec BenchmarkDB_RandomReplace-8 237 5034048 ns/op 198647 objects/sec BenchmarkDB_SequentialInsert-8 380 3048134 ns/op 328070 objects/sec ... BenchmarkDB_DeleteTracker_Baseline-8 396 3066078 ns/op 326150 objects/sec BenchmarkDB_DeleteTracker-8 169 7019558 ns/op 142459 objects/sec BenchmarkDB_RandomLookup-8 8839 137467 ns/op 7274474 objects/sec BenchmarkDB_SequentialLookup-8 8958 124483 ns/op 8033258 objects/sec BenchmarkDB_FullIteration_All-8 97218 11356 ns/op 88057271 objects/sec BenchmarkDB_FullIteration_Get-8 78102 14373 ns/op 69577187 objects/sec BenchmarkDB_PropagationDelay-8 245020 4727 ns/op 42.00 50th_µs 48.00 90th_µs 210.0 99th_µs PASS ok github.com/cilium/statedb 31.520s Signed-off-by: Jussi Maki <[email protected]>
joamaki
force-pushed
the
pr/joamaki/table-pos
branch
from
4a29918 to
7c3e0bb
Compare
pippolo84
approved these changes
Mar 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Various optimizations:
Before:
After:
Result highlights: